👋 Hi! Thank you for contributing to the vLLM project.
Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can do one of these:

• Add ready label to the PR
• Enable auto-merge.

🚀

cc @comaniac copy of #12489 not yet tried on v1, as there was issues with v1 last week.

Thanks. Will approve if it works for v1.

Thanks. Will approve if it works for v1.

sorry for delay, just got spare time to test it and V1 working :) So both works well and show throughput improvement.

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @maleksan85.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

I don't think we support the skip_tokenizer_init option yet in V1 (we should make sure there's a check on the V1 path that raises an appropriate error).

@maleksan85 WDYT about handling this in the benchmarks without using that option - in particular having explicit skip_tokenization and skip_detokenization benchmark options where the former controls whether token_ids are used instead of text prompts (per your change in this PR), and the latter is handled by #11697?

This should work in V1 now that #14224 is merged.

I don't think we support the skip_tokenizer_init option yet in V1 (we should make sure there's a check on the V1 path that raises an appropriate error).

@maleksan85 WDYT about handling this in the benchmarks without using that option - in particular having explicit skip_tokenization and skip_detokenization benchmark options where the former controls whether token_ids are used instead of text prompts (per your change in this PR), and the latter is handled by #11697?

This should work in V1 now that #14224 is merged.

This is a BUGFIX PR that restores functionality to exclude tokenize\detokenize from model inference. Tried on V1, haven't seen any errors. If you want extra flag, that vllm community was against of, when I created this PR, sure. However personally I don't see a reason for new flag(s).

/ready

@comaniac would you be able to help to land this PR?

Otherwise LGTM

great, please mark it ready then if no other concerns.

@maleksan85 apologies, LGTM too. Would it make sense to apply a similar change to the other benchmark scripts benchmark_latency.py, benchmark_prefix_caching.py, benchmark_prioritization.py?

as to what I understand benrmark_latency.py only sends ids instead of tokens. So like inherently supports skip_tokenizer_init, I mean the change I did for benchmark throughput.

For the rest of benchmarks it is up to those functionalities developers.